chore(cli): define error actionability taxonomy#5829
Conversation
…rations Defines the CLI error actionability taxonomy for KPI reporting (CLI-1560): error kinds, categories, suggestion types, and the Q2 strict-recovery / repeat-error / internal-unknown-rate metric definitions. Classification is declared where each error is defined: every error class in apps/cli exposes a CliErrorActionabilityDeclaration under the ErrorActionabilityId symbol, enforced by an exhaustiveness test that fails when any error class lacks a declaration. Errors from workspace packages (stack, config, process-compose) and effect cli/http are classified by a small structural adapter, exhaustiveness-checked against those packages' sources. Broad wrapper errors gain structured discriminants instead of message sniffing: StackBuildError.reason, DockerPullError.daemonDown, LegacyDockerRunError.reason/daemonDown, and a typed upgradeSuggested prop threaded from legacySuggestUpgrade through the plan-gated sso, branches, and vanity-subdomains errors. No telemetry capture changes: cli_command_executed still emits only existing fields until CLI-1561 wires classification into failure events. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@5b22e0cdcef64b528da57513ed54c198bcf8c4edPreview package for commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afd73d0c8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Coverage: the drift guard now also enforces declarations on plain `extends Error` classes, and the external adapters cover the full effect cli parser error set (MissingArgument, DuplicateOption, InvalidValue), UserError unwrapping, and PlatformError/BadArgument with reason-based classification. Classification fixes from a construction-site audit of all ~420 classes: local filesystem failures mislabeled as config/input errors now classify as permission; CLI-internal declarative script failures move to internal_bug; transport failures conflated with user errors in LegacyDbQueryExecError, LegacyDeclarativeApplyError, and LegacyDomainsCnameError are split via typed discriminants; paused projects get the new open_dashboard suggestion type; and LegacyDbConfigIpv6Error's suggestion now matches the link command the CLI actually prints. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68442e6671
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Coverage scan now recognizes any local `*Error("Tag")` factory (e.g. the
next login shell's LoginError), closing the drift-guard gap for NoTtyError
and LoginFailedError, which now declare auth classifications.
- classifyCliErrorActionability recurses into a StackError's preserved
tagged cause before falling back, and uses own-property adapter lookups so
sanitized tags like "constructor" cannot hit Object.prototype.
- DockerPullError.daemonDown now also covers docker spawn failures (binary
missing), not just daemon-down output.
- DeleteFunctionUnexpectedStatusError, LegacyDbQueryUnexpectedStatusError,
and LegacyLinkAuthTokenError thread the response status and classify via
statusCodeActionability, so 401s land in auth and 5xx in api_status.
- Paused projects get a dedicated project_paused category instead of
polluting plan_limit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11232cb803
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ures - classifyCliErrorActionability recurses into the classifiable preserved cause of stack wrapper errors (asset-preparation StackBuildError, DownloadError, StackError), so a daemon-down DockerPullError or a local filesystem PlatformError is no longer reported as a registry/network failure. Recursion is guarded: an unclassifiable cause keeps the wrapper's own bucket. - statusCodeActionability and the HttpClientError adapter classify 403 as user-actionable account permission (after the entitlement-gate branch, so confirmed plan gates stay plan_limit). - packages/api joins the adapter coverage scan; SupabaseApiConfigError classifies as auth/set_env_var. - LegacyStorageAuthTokenError branches on its stored status like its link twin, so api-keys 5xx responses stop counting as user login problems. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ror-actionability-colocated
The coverage guard correctly failed on the develop merge: the stop/status port and config-validation refactor introduced new error classes. Each now declares its actionability from its construction sites (workdir flags → provide_flags, config.toml content → invalid_config, docker CLI wrapper failures → docker_not_running, missing/unhealthy containers → start-stack remediation), LegacyContainerRuntimeNotFoundError is exported for guard verification, and packages/config's InvalidRemoteProjectIdError joins the external adapters as invalid_config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
From a final multi-angle review pass: - Cause-chain recursion (ShowHelp, UserError, stack wrapper causes) is capped at depth 8, so a cyclic cause chain can never stack-overflow the failure-telemetry path; regression tests cover mutual and self cycles. - Reason-less StackBuildErrors now classify their preserved cause, so a user's ProjectConfigParseError inside "Failed to configure Edge Functions" reports invalid_config instead of an internal bug; explicit invalid_config wrappers are never overridden. The cleanup-metadata persistence site drops its invalid_config reason (it is a filesystem failure, classified from its cause). - toStackError's ServiceNotFoundError case preserves `cause` like every sibling case, so wrapped stack failures classify uniformly. - DockerPullError's spawnFailed flag resets once a later attempt spawns successfully, so a transient spawn hiccup no longer reports a genuine registry failure as docker-not-running. - The docker daemon-down matcher lives once in @supabase/stack and is imported by the legacy docker-run layer; the plan-limit-gated declaration is shared (planLimitGatedActionability) instead of triplicated. - LegacyDbQueryExecError's non-transport branch classifies as dbFinding, matching every sibling "user's own SQL failed" error. - The cname failure discriminant is null-safe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…k gate The last commit left an unformatted line in packages/stack, which failed CI's Check code quality job (it runs the check targets across all projects) while the focused apps/cli checks were green. Fix the formatting and record the rule in AGENTS.md: multi-workspace changes must pass `npx nx run-many -t types:check lint:check fmt:check knip:check` from the repo root before the task is done, judged by the Nx Failed tasks list rather than filtered output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Defines the CLI error actionability taxonomy for KPI reporting (CLI-1560). Replaces #5775 with an architecture where classification lives on the error types themselves instead of a centralized tag registry.
What
apps/cli/src/shared/telemetry/error-actionability.ts):error_kind,error_category,suggestion_typeenums, the sanitizedCliErrorActionabilitycontract, and the Q2 metric definitions (strict recovery, repeat error, internal/unknown bug rate).apps/cli(391 classes) declares its own classification via a getter under theErrorActionabilityIdsymbol, next to the message/suggestion it already carries. Instance-dependent cases branch on typed fields (statusCodeActionability(this.status, { upgradeSuggested })), never on message text.TaggedErrorinapps/cli/srcand fails if one lacks a declaration (or isn't exported for verification), and checks that every error tag inpackages/{stack,config,process-compose}has an external adapter. A productionunknownclassification now means "genuinely unforeseen failure", not registry drift.StackBuildError.reason(invalid_config/asset_preparation),DockerPullError.daemonDownandLegacyDockerRunError.reason/daemonDown(detected at the docker boundary where the output is produced), and a typedupgradeSuggestedprop threaded fromlegacySuggestUpgradeinto the plan-gated sso/branches/vanity-subdomains errors — noObject.definePropertyside-channel, no sniffing user-facing English for "upgrade"/"billing"/"quota".Why the different architecture vs #5775
The centralized
actionabilityByTagmap re-stated, far from each definition, facts each error already knows about itself. It covered 229 of 407 tags after 169 review-comment rounds, and any new error silently fell through tounknown— inflating the exact KPI numerator this project is meant to measure. With co-location the classification is reviewed in the same diff that introduces an error, and the coverage test turns "we forgot to classify" into a CI failure instead of a data-quality bug.Scope notes
cli_command_executedstill emits only existing fields. Wiring classification into failure events (and command-name attribution for pre-handler failures) is CLI-1561.has_suggestion/suggestion_typefrom the code path that renders suggestions, so telemetry can never disagree with what the user saw.🤖 Generated with Claude Code